Thanks for the review! All three comments have been addressed in the latest commit:

1. Redundant inner if installed_key check (src/specify_cli/init.py): Removed the redundant inner check. The outer block already guarantees installed_key is truthy.

2. commands_dir.is_dir() guard (src/specify_cli/extensions.py): Removed the guard and replaced it with commands_dir.mkdir(parents=True, exist_ok=True) so extensions are always re-registered on switch, even if the directory does not yet exist.

3. Agent-scoped skills cleanup (src/specify_cli/extensions.py): Fixed unregister_agent_artifacts to explicitly resolve the skills directory for the provided agent_name using the same _get_skills_dir helper that extension install uses, rather than relying on the currently-active agent in init-options. Also updated _unregister_extension_skills to accept an optional skills_dir parameter.

Please address Copilot feedback. If not applicable, please explain why

Thanks for the second round of review! All three comments have been addressed:

1. Refactored test_switch_migrates_extension_commands to use the existing _run_in_project() helper instead of repeated os.chdir()/try/finally blocks, keeping the test consistent with others in the file.

2. Removed the redundant commands_dir.mkdir() call in register_enabled_extensions_for_agent() since CommandRegistrar.register_commands_for_agent() already creates the directory.

3. Updated the warning message in integration_switch() to mention both commands and skills: Could not register extension commands, skills, or related artifacts.

Addressing the two new comments:

Comment 7 (Copilot skills mode frontmatter): The concern about mode: mapping is already handled correctly. CopilotIntegration.post_process_skill_content() parses the name: field from frontmatter and converts speckit-git-feature back to speckit.git-feature format before injecting mode:. Verified locally that generated SKILL.md contains mode: speckit.git-feature.

Comment 8 (Test verification): Added assertion in test_switch_migrates_copilot_skills_extension_commands to verify the generated SKILL.md contains the correct mode: field mapping.

Addressing the two new comments:

Comment 9 (Unknown agent cleanup): Added a guard in unregister_agent_artifacts() to return early when agent_name is not present in CommandRegistrar.AGENT_CONFIGS. This prevents losing registry entries while leaving orphaned files on disk during the "unknown integration via manifest" switch path.

Comment 10 (Skills directory fallback): Changed the skills_dir passed to _unregister_extension_skills() to be None when the resolved agent_skills_dir does not exist on disk. This re-enables the fallback scan of all known agent skills directories, which is important for cleaning up stale registered_skills entries created by earlier installs.

Please address Copilot feedback

Addressing the latest Copilot comment (comment #11):

unregister_agent_artifacts() was unconditionally clearing registered_skills after calling _unregister_extension_skills(). However, _unregister_extension_skills() may intentionally skip deletion when it cannot verify ownership via metadata.source (e.g., corrupted/missing SKILL.md). This left skill directories on disk while removing the only registry reference needed for future cleanup.

Fixed by checking which skill directories still exist after cleanup and only keeping those still-present names in the registry. Future cleanup attempts will still be able to find skipped skills.

@mnriem All Copilot feedback has been addressed across multiple commits. Here's a summary of all 11 comments and their resolutions:

1. ✅ Redundant inner if installed_key check - removed
2. ✅ commands_dir.is_dir() guard preventing re-registration - replaced with mkdir()
3. ✅ Agent-scoped skills cleanup - now uses _get_skills_dir() for the specific agent
4. ✅ Test using manual os.chdir() - refactored to use _run_in_project() helper
5. ✅ Redundant commands_dir mkdir() - removed (registrar handles it)
6. ✅ Warning message accuracy - updated to mention skills/artifacts
7. ✅ Copilot skills mode: frontmatter mapping - verified correct, added test assertion
8. ✅ Test missing mode: assertion - added assertion for mode: field
9. ✅ Unknown agent cleanup guard - added AGENT_CONFIGS check to skip unsupported agents
10. ✅ Skills directory fallback - pass None when dir doesn't exist to enable fallback scan
11. ✅ Unconditional registered_skills clearing - now only removes entries for actually-deleted skills

All 244 tests pass. Please re-review when you have a chance.

Addressing the two new Copilot comments (round 4):

Comment 12 (Stale registry on empty registration): register_enabled_extensions_for_agent() now explicitly removes the agent entry from registered_commands when registration returns an empty list (e.g., corrupted manifest pointing at missing command files). This prevents stale metadata that later cleanup would try to remove.

Comment 13 (Incorrect registry reconciliation during fallback cleanup): When skills_dir is None (fallback scan mode), we no longer use agent_skills_dir to check remaining skills. Registry reconciliation now only happens when skills_dir is explicitly set (specific directory targeted), correctly matching what _unregister_extension_skills() actually removed.

All 220 tests pass (30 integration subcommand + 190 extension tests).

Fixed comment 14: Changed registry reconciliation to check (skills_dir / skill_name).is_dir() instead of SKILL.md existence. When _unregister_extension_skills() skips deletion due to missing SKILL.md (leaving directory in place), the previous check would incorrectly drop the skill from registered_skills. Now we check directory existence so skipped skills remain in the registry for future cleanup attempts.

Fixed: Updated the warning message to refer to "extension artifacts (commands, skills, registry entries)" instead of just "extension commands", since unregister_agent_artifacts() also removes skills and updates registry entries.

Thank you!